Skip to content

Convert Devenv to Nix flake#197

Merged
Schaechtle merged 12 commits into
mainfrom
srounce/convert-to-flake
Jun 5, 2024
Merged

Convert Devenv to Nix flake#197
Schaechtle merged 12 commits into
mainfrom
srounce/convert-to-flake

Conversation

@srounce

@srounce srounce commented May 28, 2024

Copy link
Copy Markdown
Collaborator
  • Remove Devenv usage, similar functionality with Nix flake allowing easier debugging and Nix REPL usage.
  • Slightly loosen Python version requirement
  • Ensure dvc cache is local to project, preventing issues on some distros that can't arbitrarily write to /var/...
  • Add Direnv config to load new devshell when entering directory
  • Enable CI on MacOS runners ensuring environment is tested on both platforms

Supercedes #194 & #195, incorporating the fixes they provide.

@srounce srounce requested review from KingMob, ships and zane May 28, 2024 10:47
@srounce srounce self-assigned this May 28, 2024
@srounce srounce force-pushed the srounce/convert-to-flake branch 3 times, most recently from 893c54c to a454f6d Compare May 28, 2024 11:19

@KingMob KingMob left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks ok afaict, but I'm not a Nix or python expert.

It looks like the Github commit-lint action is broken, though. It says it can't find pnpm. (If it helps, I generally have good luck debugging GH Actions locally with the act tool.)

@srounce

srounce commented May 28, 2024

Copy link
Copy Markdown
Collaborator Author

Yeah I'm having a look into that, it's weird because the step before also uses pnpm and works fine.

@srounce srounce force-pushed the srounce/convert-to-flake branch from a454f6d to 648edc2 Compare May 28, 2024 13:10
@KingMob

KingMob commented Jun 5, 2024

Copy link
Copy Markdown
Contributor

Looks good afaict. I noticed the .envrc is running watch_file $(find . -name '*.nix'), which means it should be looking at .devenv/ and .devenv.flake.nix. I removed them, and it still seems to be working, but should those files be ignored or deleted, to avoid interference?

@KingMob KingMob left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, tho see my note about watching all nix files recursively.

@srounce

srounce commented Jun 5, 2024

Copy link
Copy Markdown
Collaborator Author

Looks good afaict. I noticed the .envrc is running watch_file $(find . -name '*.nix'), which means it should be looking at .devenv/ and .devenv.flake.nix. I removed them, and it still seems to be working, but should those files be ignored or deleted, to avoid interference?

The Devenv dotfiles can be removed as Devenv is no longer used at all. However well pointed out as I'd forgotten to remove the old unused flake stub in the nix/ directory (now removed).

The watch_file call in the .envrc basically gets Direnv to reload the environment when there has been a change to the nix configuration as it's non-trivial to work out which nix files are related to the devshell. I'm going to add a watch_file to the flake.lock as well as that would also have potentially devshell altering effects.

srounce added 2 commits June 5, 2024 15:01
Change .envrc to use Bash file globbing rather than the find
sub-shell invocation.
@Schaechtle Schaechtle merged commit da3c2d6 into main Jun 5, 2024
@Schaechtle Schaechtle deleted the srounce/convert-to-flake branch June 5, 2024 15:30
@KingMob

KingMob commented Jun 6, 2024

Copy link
Copy Markdown
Contributor

@Schaechtle @srounce I just noticed, the docs weren't updated to match. quick-start.adoc and structure-learning.adoc still refer to devenv.

@KingMob

KingMob commented Jun 6, 2024

Copy link
Copy Markdown
Contributor

The Dockerfile also refers to devenv, and many of the Github Action yaml files do, too.

@Schaechtle Schaechtle mentioned this pull request Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants